Skip to content

sobjectizer: init at 5.8.4#403862

Merged
Sigmanificient merged 2 commits intoNixOS:masterfrom
ivalery111:sobjectizer-init
May 12, 2025
Merged

sobjectizer: init at 5.8.4#403862
Sigmanificient merged 2 commits intoNixOS:masterfrom
ivalery111:sobjectizer-init

Conversation

@ivalery111
Copy link
Copy Markdown
Member

@ivalery111 ivalery111 commented May 3, 2025

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions Bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 3, 2025
Sigmanificient
Sigmanificient requested changes May 3, 2025
View reviewed changes
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/tests/ping-pong-shared/default.nix Outdated
bengsparks
bengsparks suggested changes May 3, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@bengsparks bengsparks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requires nixfmt for CI to pass 😄

Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
@ivalery111 ivalery111 marked this pull request as draft May 4, 2025 09:47
@ivalery111 ivalery111 force-pushed the sobjectizer-init branch 2 times, most recently from 092d9cb to 79fff12 Compare May 4, 2025 10:35
@github-actions github-actions Bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label May 4, 2025
@ivalery111 ivalery111 force-pushed the sobjectizer-init branch 3 times, most recently from d0c1461 to ebd80a6 Compare May 4, 2025 12:10
bengsparks
bengsparks suggested changes May 4, 2025
View reviewed changes
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 4, 2025

@Sigmanificient @bengsparks :
Please review v2. If all good, I'll squash and rebase.
Thanks!

@ivalery111 ivalery111 marked this pull request as ready for review May 4, 2025 18:12
Sigmanificient
Sigmanificient requested changes May 4, 2025
View reviewed changes
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
bengsparks
bengsparks suggested changes May 4, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@bengsparks bengsparks left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still should incorporate this feedback so that any form of tests are run: https://github.com/NixOS/nixpkgs/pull/403862/files#r2072626252
Please resolve all completed conversations so we can see if there is anything outstanding.

You also have to split your commit into two separate ones;
One to add yourself to the maintainer-list à la maintainer-list: add ivalery111, and the second one with your new package.

I also recommend adding passthru.updateScript = nix-update-script { }; to the package so you can receive semi-automated updates.

@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 5, 2025 •
edited
Loading

You still should incorporate this feedback so that any form of tests are run: https://github.com/NixOS/nixpkgs/pull/403862/files#r2072626252 Please resolve all completed conversations so we can see if there is anything outstanding.

You also have to split your commit into two separate ones; One to add yourself to the maintainer-list à la maintainer-list: add ivalery111, and the second one with your new package.

I also recommend adding passthru.updateScript = nix-update-script { }; to the package so you can receive semi-automated updates.

$ nix-shell -p nix-update
...

[nix-shell]: $ nix-build -A sobjectizer
error:
       … while evaluating a branch condition
         at /home/v/src/github/nixpkgs/lib/customisation.nix:305:5:
          304|     in
          305|     if missingArgs == { } then
             |     ^
          306|       makeOverridable f allArgs

       … while calling the 'removeAttrs' builtin
         at /home/v/src/github/nixpkgs/lib/attrsets.nix:657:28:
          656|   */
          657|   filterAttrs = pred: set: removeAttrs set (filter (name: !pred name set.${name}) (attrNames set));
             |                            ^
          658|

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: undefined variable 'nix-update-script'
       at /home/v/src/github/nixpkgs/pkgs/by-name/so/sobjectizer/package.nix:39:27:
           38|
           39|   passthru.updateScript = nix-update-script { };
             |                           ^
           40|

Sadness...

UPD:
All good with pkgs.nix-update-script { }; :)

bengsparks
bengsparks suggested changes May 5, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@bengsparks bengsparks left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the build system of this project and found that the static library is not needed to run the tests. I've submitted Stiffstream/sobjectizer#95, which you can make use of with fetchpatch.

Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
bengsparks
bengsparks suggested changes May 5, 2025
View reviewed changes
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
@ivalery111 ivalery111 marked this pull request as draft May 6, 2025 20:15
@ofborg ofborg Bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels May 6, 2025
@ofborg ofborg Bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 6, 2025
@github-actions github-actions Bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 6, 2025
@github-actions github-actions Bot removed 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: rust General-purpose programming language emphasizing performance, type safety, and concurrency. 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: ruby A dynamic, open source programming language with a focus on simplicity and productivity. 6.topic: vim Advanced text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: module system About "NixOS" module system internals 6.topic: agda A dependently typed programming language / interactive theorem prover 6.topic: java Including JDK, tooling, other languages, other VMs 6.topic: nim Nim programing language 6.topic: flakes The experimental Nix feature 6.topic: lib The Nixpkgs function library labels May 7, 2025
@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 7, 2025

@bengsparks :

Please take a look.
Thanks.

bengsparks
bengsparks suggested changes May 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@bengsparks bengsparks left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job on fixing your history 😄 looks like the final round of tweaks, then this can be left to merge 🚀
Don't forget to squash your third commit into the second one.

Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
Comment thread pkgs/by-name/so/sobjectizer/package.nix Outdated
ivalery111 added 2 commits May 7, 2025 20:36
@ivalery111
maintainer-list: add ivalery111
45087aa 
Signed-off-by: Valery Ivanov <[email protected]>
@ivalery111
sobjectizer: init at 5.8.4
2c503c3 
Signed-off-by: Valery Ivanov <[email protected]>
@bengsparks
Copy link
Copy Markdown
Contributor

bengsparks commented May 7, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 403862

x86_64-linux

✅ 1 package built:
  • sobjectizer

aarch64-linux

✅ 1 package built:
  • sobjectizer

x86_64-darwin

✅ 1 package built:
  • sobjectizer

aarch64-darwin

✅ 1 package built:
  • sobjectizer

@bengsparks
Copy link
Copy Markdown
Contributor

bengsparks commented May 7, 2025

@Sigmanificient anything to add?

Sigmanificient
Sigmanificient approved these changes May 11, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@Sigmanificient Sigmanificient left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake is not my domain of expertise 😅

@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 12, 2025

cmake is not my domain of expertise 😅

Anyway, thanks for the review :)

@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 12, 2025

@Sigmanificient :
Can you do the merge?

@ivalery111
Copy link
Copy Markdown
Member Author

ivalery111 commented May 12, 2025 •
edited
Loading

@bengsparks :
Thanks a lot for your cooperation!

P.S. There are also extensions for the SObjectizer - so5extra. When or if I'll work on this, I'll ping you :) Joke...or not... :)

Anyway, thank you and good luck!

@bengsparks
Copy link
Copy Markdown
Contributor

bengsparks commented May 12, 2025

Feel free to tag me when you do 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sigmanificient Sigmanificient Sigmanificient approved these changes

+2 more reviewers

@nicolas-goudry nicolas-goudry nicolas-goudry left review comments

@bengsparks bengsparks bengsparks approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ivalery111 @bengsparks @nicolas-goudry @Sigmanificient @wegank